[clang-tidy][C++20] Add support for Initialization Forwarding in structs and Nested Objects within modernize-use-emplace #131969

RiverDave · 2025-03-19T03:50:39Z

Aims to address: #115841

C++20 introduced argument forwarding for temporal objects(either aggregates or struct types with constructor initialization) within emplace_back calls. I've applied similar rules as they were used in constructor expressions when called with c++20. I'd love to get some feedback related to my test coverage and more possible edge cases I might've missed.

Note: C++20 introduced functional initialization/forwarding as well, this could be targeted in a different PR.

llvmbot · 2025-03-19T03:51:18Z

@llvm/pr-subscribers-clang-tools-extra

Author: David Rivera (RiverDave)

Changes

Aims to address: #115841

C++20 introduced in-place initialization for aggregate types within emplace_back calls. I ensured to match any braced init expressions with temporals (and applied similar rules as they were used in constructor expressions) when called with c++20. I'd love to get some feedback related to my test coverage and more possible edge cases I might've missed.

Full diff: https://github.com/llvm/llvm-project/pull/131969.diff

4 Files Affected:

(modified) clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp (+49-21)
(modified) clang-tools-extra/docs/ReleaseNotes.rst (+4)
(added) clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace-cxx20.cpp (+86)
(modified) clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp (+1-1)

diff --git a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
index 430455a38f395..8c42b3a8a6e3f 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
@@ -98,8 +98,8 @@ auto hasWantedType(llvm::ArrayRef<StringRef> TypeNames) {
 
 // Matches member call expressions of the named method on the listed container
 // types.
-auto cxxMemberCallExprOnContainer(
-    StringRef MethodName, llvm::ArrayRef<StringRef> ContainerNames) {
+auto cxxMemberCallExprOnContainer(StringRef MethodName,
+                                  llvm::ArrayRef<StringRef> ContainerNames) {
   return cxxMemberCallExpr(
       hasDeclaration(functionDecl(hasName(MethodName))),
       on(hasTypeOrPointeeType(hasWantedType(ContainerNames))));
@@ -174,19 +174,19 @@ void UseEmplaceCheck::registerMatchers(MatchFinder *Finder) {
   // passed pointer because smart pointer won't be constructed
   // (and destructed) as in push_back case.
   auto IsCtorOfSmartPtr =
-      hasDeclaration(cxxConstructorDecl(ofClass(hasAnyName(SmartPointers))));
+      cxxConstructorDecl(ofClass(hasAnyName(SmartPointers)));
 
   // Bitfields binds only to consts and emplace_back take it by universal ref.
-  auto BitFieldAsArgument = hasAnyArgument(
-      ignoringImplicit(memberExpr(hasDeclaration(fieldDecl(isBitField())))));
+  auto BitFieldAsArgument =
+      ignoringImplicit(memberExpr(hasDeclaration(fieldDecl(isBitField()))));
 
   // Initializer list can't be passed to universal reference.
-  auto InitializerListAsArgument = hasAnyArgument(
+  auto InitializerListAsArgument =
       ignoringImplicit(allOf(cxxConstructExpr(isListInitialization()),
-                             unless(cxxTemporaryObjectExpr()))));
+                             unless(cxxTemporaryObjectExpr())));
 
   // We could have leak of resource.
-  auto NewExprAsArgument = hasAnyArgument(ignoringImplicit(cxxNewExpr()));
+  auto NewExprAsArgument = ignoringImplicit(cxxNewExpr());
   // We would call another constructor.
   auto ConstructingDerived =
       hasParent(implicitCastExpr(hasCastKind(CastKind::CK_DerivedToBase)));
@@ -202,19 +202,36 @@ void UseEmplaceCheck::registerMatchers(MatchFinder *Finder) {
   // overloaded functions and template names.
   auto SoughtConstructExpr =
       cxxConstructExpr(
-          unless(anyOf(IsCtorOfSmartPtr, HasInitList, BitFieldAsArgument,
-                       InitializerListAsArgument, NewExprAsArgument,
-                       ConstructingDerived, IsPrivateOrProtectedCtor)))
+          unless(anyOf(hasDeclaration(IsCtorOfSmartPtr), HasInitList,
+                       hasAnyArgument(BitFieldAsArgument),
+                       hasAnyArgument(InitializerListAsArgument),
+                       hasAnyArgument(NewExprAsArgument), ConstructingDerived,
+                       IsPrivateOrProtectedCtor)))
           .bind("ctor");
-  auto HasConstructExpr = has(ignoringImplicit(SoughtConstructExpr));
+
+  auto IsPrimitiveType = hasType(builtinType());
+
+  auto AggregateInitExpr =
+      getLangOpts().CPlusPlus20
+          ? initListExpr(unless(anyOf(HasInitList, has(IsCtorOfSmartPtr),
+                                      has(BitFieldAsArgument),
+                                      has(InitializerListAsArgument),
+                                      has(NewExprAsArgument), IsPrimitiveType)))
+                .bind("agg_init")
+          : unless(anything());
+
+  auto HasConstructExpr =
+      has(ignoringImplicit(anyOf(SoughtConstructExpr, AggregateInitExpr)));
 
   // allow for T{} to be replaced, even if no CTOR is declared
   auto HasConstructInitListExpr = has(initListExpr(
-      initCountLeq(1), anyOf(allOf(has(SoughtConstructExpr),
-                                   has(cxxConstructExpr(argumentCountIs(0)))),
-                             has(cxxBindTemporaryExpr(
-                                 has(SoughtConstructExpr),
-                                 has(cxxConstructExpr(argumentCountIs(0))))))));
+      initCountLeq(1),
+      anyOf(allOf(has(SoughtConstructExpr),
+                  has(cxxConstructExpr(argumentCountIs(0)))),
+            has(cxxBindTemporaryExpr(has(SoughtConstructExpr),
+                                     has(cxxConstructExpr(argumentCountIs(0)))
+
+                                         )))));
   auto HasBracedInitListExpr =
       anyOf(has(cxxBindTemporaryExpr(HasConstructInitListExpr)),
             HasConstructInitListExpr);
@@ -314,6 +331,7 @@ void UseEmplaceCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *EmplacyCall =
       Result.Nodes.getNodeAs<CXXMemberCallExpr>("emplacy_call");
   const auto *CtorCall = Result.Nodes.getNodeAs<CXXConstructExpr>("ctor");
+  const auto *AggInitCall = Result.Nodes.getNodeAs<InitListExpr>("agg_init");
   const auto *MakeCall = Result.Nodes.getNodeAs<CallExpr>("make");
   const auto *TemporaryExpr =
       Result.Nodes.getNodeAs<MaterializeTemporaryExpr>("temporary_expr");
@@ -332,12 +350,19 @@ void UseEmplaceCheck::check(const MatchFinder::MatchResult &Result) {
   }();
 
   assert(Call && "No call matched");
-  assert((CtorCall || MakeCall) && "No push_back parameter matched");
+  assert((CtorCall || MakeCall || AggInitCall) &&
+         "No push_back parameter matched");
 
   if (IgnoreImplicitConstructors && CtorCall && CtorCall->getNumArgs() >= 1 &&
       CtorCall->getArg(0)->getSourceRange() == CtorCall->getSourceRange())
     return;
 
+  if (IgnoreImplicitConstructors && AggInitCall &&
+      AggInitCall->getNumInits() >= 1 &&
+      AggInitCall->getInit(0)->getSourceRange() ==
+          AggInitCall->getSourceRange())
+    return;
+
   const auto FunctionNameSourceRange = CharSourceRange::getCharRange(
       Call->getExprLoc(), Call->getArg(0)->getExprLoc());
 
@@ -345,6 +370,7 @@ void UseEmplaceCheck::check(const MatchFinder::MatchResult &Result) {
       EmplacyCall
           ? diag(TemporaryExpr ? TemporaryExpr->getBeginLoc()
                  : CtorCall    ? CtorCall->getBeginLoc()
+                 : AggInitCall ? AggInitCall->getBeginLoc()
                                : MakeCall->getBeginLoc(),
                  "unnecessary temporary object created while calling %0")
           : diag(Call->getExprLoc(), "use emplace%select{|_back|_front}0 "
@@ -376,9 +402,10 @@ void UseEmplaceCheck::check(const MatchFinder::MatchResult &Result) {
   }
 
   const SourceRange CallParensRange =
-      MakeCall ? SourceRange(MakeCall->getCallee()->getEndLoc(),
-                             MakeCall->getRParenLoc())
-               : CtorCall->getParenOrBraceRange();
+      MakeCall   ? SourceRange(MakeCall->getCallee()->getEndLoc(),
+                               MakeCall->getRParenLoc())
+      : CtorCall ? CtorCall->getParenOrBraceRange()
+                 : AggInitCall->getSourceRange();
 
   // Finish if there is no explicit constructor call.
   if (CallParensRange.getBegin().isInvalid())
@@ -387,6 +414,7 @@ void UseEmplaceCheck::check(const MatchFinder::MatchResult &Result) {
   // FIXME: Will there ever be a CtorCall, if there is no TemporaryExpr?
   const SourceLocation ExprBegin = TemporaryExpr ? TemporaryExpr->getExprLoc()
                                    : CtorCall    ? CtorCall->getExprLoc()
+                                   : AggInitCall ? AggInitCall->getExprLoc()
                                                  : MakeCall->getExprLoc();
 
   // Range for constructor name and opening brace.
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 72aa05eb4dcd1..125d45ca54c34 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -171,6 +171,10 @@ Changes in existing checks
   ``constexpr`` and ``static``` values on member initialization and by detecting
   explicit casting of built-in types within member list initialization.
 
+- Improved :doc:`modernize-use-emplace
+  <clang-tidy/checks/modernize/use-emplace>` check by adding support for C++20
+  aggregate initialization.
+
 - Improved :doc:`modernize-use-ranges
   <clang-tidy/checks/modernize/use-ranges>` check by updating suppress 
   warnings logic for ``nullptr`` in ``std::find``.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace-cxx20.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace-cxx20.cpp
new file mode 100644
index 0000000000000..5b0aa76dc8ab8
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace-cxx20.cpp
@@ -0,0 +1,86 @@
+// RUN: %check_clang_tidy %s -std=c++20 modernize-use-emplace %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN:             {modernize-use-emplace.ContainersWithPushBack: \
+// RUN:                '::std::vector; ::std::list; ::std::deque; llvm::LikeASmallVector', \
+// RUN:              modernize-use-emplace.TupleTypes: \
+// RUN:                '::std::pair; std::tuple; ::test::Single', \
+// RUN:              modernize-use-emplace.TupleMakeFunctions: \
+// RUN:                '::std::make_pair; ::std::make_tuple; ::test::MakeSingle'}}"
+
+namespace std {
+template <typename E>
+class initializer_list {
+public:
+  const E *a, *b;
+  initializer_list() noexcept {}
+};
+
+template <typename T>
+class vector {
+public:
+  using value_type = T;
+
+  class iterator {};
+  class const_iterator {};
+  const_iterator begin() { return const_iterator{}; }
+
+  vector() = default;
+  vector(initializer_list<T>) {}
+
+  void push_back(const T &) {}
+  void push_back(T &&) {}
+
+  template <typename... Args>
+  void emplace_back(Args &&... args){};
+  template <typename... Args>
+  iterator emplace(const_iterator pos, Args &&...args);
+  ~vector();
+};
+
+} // namespace std
+
+struct InnerType {
+  InnerType() {}
+  InnerType(char const*) {}
+};
+
+//Not aggregate but we should still be able to directly initialize it with emplace_back
+struct NonTrivialNoCtor {
+  InnerType it;
+};
+
+struct Aggregate {
+  int a;
+  int b;
+};
+
+void testCXX20Cases() {
+  std::vector<Aggregate> v1;
+
+  v1.push_back(Aggregate{1, 2});
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back instead of push_back
+  // CHECK-FIXES: v1.emplace_back(1, 2);
+
+  v1.push_back({1, 2});
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back instead of push_back
+  // CHECK-FIXES: v1.emplace_back(1, 2);
+
+  std::vector<NonTrivialNoCtor> v2;
+
+  v2.push_back(NonTrivialNoCtor{""});
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back instead of push_back
+  // CHECK-FIXES: v2.emplace_back("");
+
+  v2.push_back({""});
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back instead of push_back
+  // CHECK-FIXES: v2.emplace_back("");
+
+  v2.push_back(NonTrivialNoCtor{InnerType{""}});
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back instead of push_back
+  // CHECK-FIXES: v2.emplace_back(InnerType{""});
+
+  v2.push_back({InnerType{""}});
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back instead of push_back
+  // CHECK-FIXES: v2.emplace_back(InnerType{""});
+
+}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp
index e6562cd18dbab..f2390f38c3166 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s modernize-use-emplace %t -- \
+// RUN: %check_clang_tidy %s -std=c++17 modernize-use-emplace %t -- \
 // RUN:   -config="{CheckOptions: \
 // RUN:             {modernize-use-emplace.ContainersWithPushBack: \
 // RUN:                '::std::vector; ::std::list; ::std::deque; llvm::LikeASmallVector', \

llvmbot · 2025-03-19T03:51:19Z

@llvm/pr-subscribers-clang-tidy

Author: David Rivera (RiverDave)

Changes

Aims to address: #115841

C++20 introduced in-place initialization for aggregate types within emplace_back calls. I ensured to match any braced init expressions with temporals (and applied similar rules as they were used in constructor expressions) when called with c++20. I'd love to get some feedback related to my test coverage and more possible edge cases I might've missed.

Full diff: https://github.com/llvm/llvm-project/pull/131969.diff

4 Files Affected:

(modified) clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp (+49-21)
(modified) clang-tools-extra/docs/ReleaseNotes.rst (+4)
(added) clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace-cxx20.cpp (+86)
(modified) clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp (+1-1)

diff --git a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
index 430455a38f395..8c42b3a8a6e3f 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
@@ -98,8 +98,8 @@ auto hasWantedType(llvm::ArrayRef<StringRef> TypeNames) {
 
 // Matches member call expressions of the named method on the listed container
 // types.
-auto cxxMemberCallExprOnContainer(
-    StringRef MethodName, llvm::ArrayRef<StringRef> ContainerNames) {
+auto cxxMemberCallExprOnContainer(StringRef MethodName,
+                                  llvm::ArrayRef<StringRef> ContainerNames) {
   return cxxMemberCallExpr(
       hasDeclaration(functionDecl(hasName(MethodName))),
       on(hasTypeOrPointeeType(hasWantedType(ContainerNames))));
@@ -174,19 +174,19 @@ void UseEmplaceCheck::registerMatchers(MatchFinder *Finder) {
   // passed pointer because smart pointer won't be constructed
   // (and destructed) as in push_back case.
   auto IsCtorOfSmartPtr =
-      hasDeclaration(cxxConstructorDecl(ofClass(hasAnyName(SmartPointers))));
+      cxxConstructorDecl(ofClass(hasAnyName(SmartPointers)));
 
   // Bitfields binds only to consts and emplace_back take it by universal ref.
-  auto BitFieldAsArgument = hasAnyArgument(
-      ignoringImplicit(memberExpr(hasDeclaration(fieldDecl(isBitField())))));
+  auto BitFieldAsArgument =
+      ignoringImplicit(memberExpr(hasDeclaration(fieldDecl(isBitField()))));
 
   // Initializer list can't be passed to universal reference.
-  auto InitializerListAsArgument = hasAnyArgument(
+  auto InitializerListAsArgument =
       ignoringImplicit(allOf(cxxConstructExpr(isListInitialization()),
-                             unless(cxxTemporaryObjectExpr()))));
+                             unless(cxxTemporaryObjectExpr())));
 
   // We could have leak of resource.
-  auto NewExprAsArgument = hasAnyArgument(ignoringImplicit(cxxNewExpr()));
+  auto NewExprAsArgument = ignoringImplicit(cxxNewExpr());
   // We would call another constructor.
   auto ConstructingDerived =
       hasParent(implicitCastExpr(hasCastKind(CastKind::CK_DerivedToBase)));
@@ -202,19 +202,36 @@ void UseEmplaceCheck::registerMatchers(MatchFinder *Finder) {
   // overloaded functions and template names.
   auto SoughtConstructExpr =
       cxxConstructExpr(
-          unless(anyOf(IsCtorOfSmartPtr, HasInitList, BitFieldAsArgument,
-                       InitializerListAsArgument, NewExprAsArgument,
-                       ConstructingDerived, IsPrivateOrProtectedCtor)))
+          unless(anyOf(hasDeclaration(IsCtorOfSmartPtr), HasInitList,
+                       hasAnyArgument(BitFieldAsArgument),
+                       hasAnyArgument(InitializerListAsArgument),
+                       hasAnyArgument(NewExprAsArgument), ConstructingDerived,
+                       IsPrivateOrProtectedCtor)))
           .bind("ctor");
-  auto HasConstructExpr = has(ignoringImplicit(SoughtConstructExpr));
+
+  auto IsPrimitiveType = hasType(builtinType());
+
+  auto AggregateInitExpr =
+      getLangOpts().CPlusPlus20
+          ? initListExpr(unless(anyOf(HasInitList, has(IsCtorOfSmartPtr),
+                                      has(BitFieldAsArgument),
+                                      has(InitializerListAsArgument),
+                                      has(NewExprAsArgument), IsPrimitiveType)))
+                .bind("agg_init")
+          : unless(anything());
+
+  auto HasConstructExpr =
+      has(ignoringImplicit(anyOf(SoughtConstructExpr, AggregateInitExpr)));
 
   // allow for T{} to be replaced, even if no CTOR is declared
   auto HasConstructInitListExpr = has(initListExpr(
-      initCountLeq(1), anyOf(allOf(has(SoughtConstructExpr),
-                                   has(cxxConstructExpr(argumentCountIs(0)))),
-                             has(cxxBindTemporaryExpr(
-                                 has(SoughtConstructExpr),
-                                 has(cxxConstructExpr(argumentCountIs(0))))))));
+      initCountLeq(1),
+      anyOf(allOf(has(SoughtConstructExpr),
+                  has(cxxConstructExpr(argumentCountIs(0)))),
+            has(cxxBindTemporaryExpr(has(SoughtConstructExpr),
+                                     has(cxxConstructExpr(argumentCountIs(0)))
+
+                                         )))));
   auto HasBracedInitListExpr =
       anyOf(has(cxxBindTemporaryExpr(HasConstructInitListExpr)),
             HasConstructInitListExpr);
@@ -314,6 +331,7 @@ void UseEmplaceCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *EmplacyCall =
       Result.Nodes.getNodeAs<CXXMemberCallExpr>("emplacy_call");
   const auto *CtorCall = Result.Nodes.getNodeAs<CXXConstructExpr>("ctor");
+  const auto *AggInitCall = Result.Nodes.getNodeAs<InitListExpr>("agg_init");
   const auto *MakeCall = Result.Nodes.getNodeAs<CallExpr>("make");
   const auto *TemporaryExpr =
       Result.Nodes.getNodeAs<MaterializeTemporaryExpr>("temporary_expr");
@@ -332,12 +350,19 @@ void UseEmplaceCheck::check(const MatchFinder::MatchResult &Result) {
   }();
 
   assert(Call && "No call matched");
-  assert((CtorCall || MakeCall) && "No push_back parameter matched");
+  assert((CtorCall || MakeCall || AggInitCall) &&
+         "No push_back parameter matched");
 
   if (IgnoreImplicitConstructors && CtorCall && CtorCall->getNumArgs() >= 1 &&
       CtorCall->getArg(0)->getSourceRange() == CtorCall->getSourceRange())
     return;
 
+  if (IgnoreImplicitConstructors && AggInitCall &&
+      AggInitCall->getNumInits() >= 1 &&
+      AggInitCall->getInit(0)->getSourceRange() ==
+          AggInitCall->getSourceRange())
+    return;
+
   const auto FunctionNameSourceRange = CharSourceRange::getCharRange(
       Call->getExprLoc(), Call->getArg(0)->getExprLoc());
 
@@ -345,6 +370,7 @@ void UseEmplaceCheck::check(const MatchFinder::MatchResult &Result) {
       EmplacyCall
           ? diag(TemporaryExpr ? TemporaryExpr->getBeginLoc()
                  : CtorCall    ? CtorCall->getBeginLoc()
+                 : AggInitCall ? AggInitCall->getBeginLoc()
                                : MakeCall->getBeginLoc(),
                  "unnecessary temporary object created while calling %0")
           : diag(Call->getExprLoc(), "use emplace%select{|_back|_front}0 "
@@ -376,9 +402,10 @@ void UseEmplaceCheck::check(const MatchFinder::MatchResult &Result) {
   }
 
   const SourceRange CallParensRange =
-      MakeCall ? SourceRange(MakeCall->getCallee()->getEndLoc(),
-                             MakeCall->getRParenLoc())
-               : CtorCall->getParenOrBraceRange();
+      MakeCall   ? SourceRange(MakeCall->getCallee()->getEndLoc(),
+                               MakeCall->getRParenLoc())
+      : CtorCall ? CtorCall->getParenOrBraceRange()
+                 : AggInitCall->getSourceRange();
 
   // Finish if there is no explicit constructor call.
   if (CallParensRange.getBegin().isInvalid())
@@ -387,6 +414,7 @@ void UseEmplaceCheck::check(const MatchFinder::MatchResult &Result) {
   // FIXME: Will there ever be a CtorCall, if there is no TemporaryExpr?
   const SourceLocation ExprBegin = TemporaryExpr ? TemporaryExpr->getExprLoc()
                                    : CtorCall    ? CtorCall->getExprLoc()
+                                   : AggInitCall ? AggInitCall->getExprLoc()
                                                  : MakeCall->getExprLoc();
 
   // Range for constructor name and opening brace.
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 72aa05eb4dcd1..125d45ca54c34 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -171,6 +171,10 @@ Changes in existing checks
   ``constexpr`` and ``static``` values on member initialization and by detecting
   explicit casting of built-in types within member list initialization.
 
+- Improved :doc:`modernize-use-emplace
+  <clang-tidy/checks/modernize/use-emplace>` check by adding support for C++20
+  aggregate initialization.
+
 - Improved :doc:`modernize-use-ranges
   <clang-tidy/checks/modernize/use-ranges>` check by updating suppress 
   warnings logic for ``nullptr`` in ``std::find``.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace-cxx20.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace-cxx20.cpp
new file mode 100644
index 0000000000000..5b0aa76dc8ab8
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace-cxx20.cpp
@@ -0,0 +1,86 @@
+// RUN: %check_clang_tidy %s -std=c++20 modernize-use-emplace %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN:             {modernize-use-emplace.ContainersWithPushBack: \
+// RUN:                '::std::vector; ::std::list; ::std::deque; llvm::LikeASmallVector', \
+// RUN:              modernize-use-emplace.TupleTypes: \
+// RUN:                '::std::pair; std::tuple; ::test::Single', \
+// RUN:              modernize-use-emplace.TupleMakeFunctions: \
+// RUN:                '::std::make_pair; ::std::make_tuple; ::test::MakeSingle'}}"
+
+namespace std {
+template <typename E>
+class initializer_list {
+public:
+  const E *a, *b;
+  initializer_list() noexcept {}
+};
+
+template <typename T>
+class vector {
+public:
+  using value_type = T;
+
+  class iterator {};
+  class const_iterator {};
+  const_iterator begin() { return const_iterator{}; }
+
+  vector() = default;
+  vector(initializer_list<T>) {}
+
+  void push_back(const T &) {}
+  void push_back(T &&) {}
+
+  template <typename... Args>
+  void emplace_back(Args &&... args){};
+  template <typename... Args>
+  iterator emplace(const_iterator pos, Args &&...args);
+  ~vector();
+};
+
+} // namespace std
+
+struct InnerType {
+  InnerType() {}
+  InnerType(char const*) {}
+};
+
+//Not aggregate but we should still be able to directly initialize it with emplace_back
+struct NonTrivialNoCtor {
+  InnerType it;
+};
+
+struct Aggregate {
+  int a;
+  int b;
+};
+
+void testCXX20Cases() {
+  std::vector<Aggregate> v1;
+
+  v1.push_back(Aggregate{1, 2});
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back instead of push_back
+  // CHECK-FIXES: v1.emplace_back(1, 2);
+
+  v1.push_back({1, 2});
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back instead of push_back
+  // CHECK-FIXES: v1.emplace_back(1, 2);
+
+  std::vector<NonTrivialNoCtor> v2;
+
+  v2.push_back(NonTrivialNoCtor{""});
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back instead of push_back
+  // CHECK-FIXES: v2.emplace_back("");
+
+  v2.push_back({""});
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back instead of push_back
+  // CHECK-FIXES: v2.emplace_back("");
+
+  v2.push_back(NonTrivialNoCtor{InnerType{""}});
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back instead of push_back
+  // CHECK-FIXES: v2.emplace_back(InnerType{""});
+
+  v2.push_back({InnerType{""}});
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back instead of push_back
+  // CHECK-FIXES: v2.emplace_back(InnerType{""});
+
+}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp
index e6562cd18dbab..f2390f38c3166 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s modernize-use-emplace %t -- \
+// RUN: %check_clang_tidy %s -std=c++17 modernize-use-emplace %t -- \
 // RUN:   -config="{CheckOptions: \
 // RUN:             {modernize-use-emplace.ContainersWithPushBack: \
 // RUN:                '::std::vector; ::std::list; ::std::deque; llvm::LikeASmallVector', \

clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp

clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace-cxx20.cpp

clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp

github-actions · 2025-03-24T05:00:17Z

✅ With the latest revision this PR passed the C/C++ code formatter.

RiverDave · 2025-03-24T05:09:36Z

Removed "Aggregates" from both title and release notes considering my changes can potentially affect non aggregate structs types as noticed in regression tests.

RiverDave · 2025-03-27T13:27:48Z

Ping

vbvictor

Few nits.
Thought, I'm not very familiar this kind of AST-matchers and PR needs review from someone else.

clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp

clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp

clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp

RiverDave · 2025-04-05T01:39:02Z

Ping

RiverDave · 2025-04-18T15:16:06Z

Ping @piotrdz @5chmidti @HerrCai0907 :)

…cts within modernize-use-emplace

RiverDave · 2025-05-03T04:46:50Z

Ping

vbvictor · 2025-07-14T06:32:24Z

clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp

@@ -1,4 +1,12 @@
-// RUN: %check_clang_tidy %s modernize-use-emplace %t -- \
+// RUN: %check_clang_tidy %s -std=c++11 modernize-use-emplace %t -- \


Suggested change

// RUN: %check_clang_tidy %s -std=c++11 modernize-use-emplace %t -- \

// RUN: %check_clang_tidy %s -std=c++11,c++14,c++17 modernize-use-emplace %t -- \

vbvictor · 2025-07-14T06:32:34Z

clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp

+// RUN:                '::std::pair; std::tuple; ::test::Single', \
+// RUN:              modernize-use-emplace.TupleMakeFunctions: \
+// RUN:                '::std::make_pair; ::std::make_tuple; ::test::MakeSingle'}}"
+// RUN: %check_clang_tidy -std=c++20 %s modernize-use-emplace -check-suffixes=,CPP20 %t  -- \


Suggested change

// RUN: %check_clang_tidy -std=c++20 %s modernize-use-emplace -check-suffixes=,CPP20 %t -- \

// RUN: %check_clang_tidy -std=c++20-or-later %s modernize-use-emplace -check-suffixes=,CPP20 %t -- \

vbvictor · 2025-07-14T06:33:42Z

clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp

+    return;
+
+  if (getLangOpts().LangStd >= LangStandard::lang_cxx20 && AggInitCall) {
+    for (const auto *Init : AggInitCall->inits()) {


Please use real type instead of auto

vbvictor · 2025-07-14T06:35:09Z

clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp

+      if (const auto *InnerConstructorExpr = unwrapToConstructorExpr(Init)) {
+        // consume all args if it's an empty constructor call so that we can ->
+        // T{} -> emplace_back()
+        if (InnerConstructorExpr && InnerConstructorExpr->getNumArgs() == 0) {


Suggested change

if (InnerConstructorExpr && InnerConstructorExpr->getNumArgs() == 0) {

if (InnerConstructorExpr->getNumArgs() == 0) {

No need to check that InnerConstructorExpr is not null here because it's done in previous if

vbvictor · 2025-07-14T06:36:33Z

clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp

+  v2.push_back({std::vector<int>{0}});
+  // CHECK-MESSAGES-CPP20: :[[@LINE-1]]:6: warning: use emplace_back instead of push_back
+  // CHECK-FIXES-CPP20: v2.emplace_back(std::vector<int>{0});
+  }


Suggested change

}

}

vbvictor · 2025-07-14T06:41:18Z

clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp

  // CHECK-FIXES: sp->emplace();
 }
+
+namespace GH1225740 {


nit: After N years this could be useless, I'd suggest removing it

We have added markers like these. Though I don't have a strong opinion on whether we need them since the git log would link to the PR and therefore resolved issues.

TBH, older markers (especially from pre-GitHub era) seems like white noise to me. Usually you can get from the context what the author wanted to say.

This strikes me as code with no version control, where people would put date and description directly in source code🙃

vbvictor · 2025-07-14T06:43:39Z

clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp

+
+  if (getLangOpts().LangStd >= LangStandard::lang_cxx20 && AggInitCall) {
+    for (const auto *Init : AggInitCall->inits()) {
+      if (const auto *InnerConstructorExpr = unwrapToConstructorExpr(Init)) {


Could you use Expr::IgnoreImplicit instead of unwrapToConstructorExpr. Looking at its code, it must do the same.

vbvictor · 2025-07-14T06:44:55Z

clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp

+
+namespace GH1225740 {
+
+void CXX20testBracedInitTemporaries(){


Suggested change

void CXX20testBracedInitTemporaries(){

void CXX20testBracedInitTemporaries() {

vbvictor · 2025-07-14T06:48:21Z

clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp

+namespace GH1225740 {
+
+void CXX20testBracedInitTemporaries(){
+


Could we add tests with multiple values in initListExpr(). AFAIK we only have tests with 0 or 1 values.

vbvictor · 2025-07-14T06:48:54Z

clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp

+
+  if (getLangOpts().LangStd >= LangStandard::lang_cxx20 && AggInitCall) {
+    for (const auto *Init : AggInitCall->inits()) {
+      if (const auto *InnerConstructorExpr = unwrapToConstructorExpr(Init)) {


Please use real type instead of auto

5chmidti · 2025-07-13T18:27:38Z

clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp

+          AggInitCall->getSourceRange())
+    return;
+
+  if (getLangOpts().LangStd >= LangStandard::lang_cxx20 && AggInitCall) {


getLangOpts().CPlusPlus20 && ...

5chmidti · 2025-07-14T20:55:56Z

clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp

  // CHECK-FIXES: sp->emplace();
 }
+
+namespace GH1225740 {


We have added markers like these. Though I don't have a strong opinion on whether we need them since the git log would link to the PR and therefore resolved issues.

llvmbot added clang-tools-extra clang-tidy labels Mar 19, 2025

RiverDave changed the title ~~[clang-tidy] Add support for aggregate types within modernize-use-emplace~~ [clang-tidy][C++20] Add support for aggregate types within modernize-use-emplace Mar 19, 2025

vbvictor reviewed Mar 19, 2025

View reviewed changes

clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp Outdated Show resolved Hide resolved

vbvictor reviewed Mar 19, 2025

View reviewed changes

clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace-cxx20.cpp Outdated Show resolved Hide resolved

vbvictor reviewed Mar 19, 2025

View reviewed changes

clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace-cxx20.cpp Outdated Show resolved Hide resolved

vbvictor reviewed Mar 19, 2025

View reviewed changes

clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp Outdated Show resolved Hide resolved

EugeneZelenko requested review from 5chmidti, HerrCai0907 and PiotrZSL March 19, 2025 13:58

RiverDave changed the title ~~[clang-tidy][C++20] Add support for aggregate types within modernize-use-emplace~~ [clang-tidy][C++20] Add support for Initialization Forwarding in Nested Object Construction Mar 24, 2025

RiverDave force-pushed the modernize/125740 branch 2 times, most recently from c966cef to fa0f107 Compare March 24, 2025 04:55

RiverDave force-pushed the modernize/125740 branch from fa0f107 to 30678d7 Compare March 24, 2025 05:01

RiverDave changed the title ~~[clang-tidy][C++20] Add support for Initialization Forwarding in Nested Object Construction~~ [clang-tidy][C++20] Add support for Initialization Forwarding in Nested Objects Mar 24, 2025

vbvictor reviewed Mar 28, 2025

View reviewed changes

RiverDave changed the title ~~[clang-tidy][C++20] Add support for Initialization Forwarding in Nested Objects~~ [clang-tidy][C++20] Add support for Initialization Forwarding in structs and Nested Objects within modernize-use-emplace Mar 29, 2025

RiverDave force-pushed the modernize/125740 branch 2 times, most recently from 9972aa8 to f0b31f8 Compare March 29, 2025 18:52

vbvictor reviewed Mar 30, 2025

View reviewed changes

clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp Outdated Show resolved Hide resolved

vbvictor reviewed Mar 30, 2025

View reviewed changes

clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp Outdated Show resolved Hide resolved

vbvictor reviewed Mar 30, 2025

View reviewed changes

clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp Outdated Show resolved Hide resolved

RiverDave force-pushed the modernize/125740 branch 3 times, most recently from 89412d9 to 27f7da4 Compare April 2, 2025 03:48

RiverDave force-pushed the modernize/125740 branch from 27f7da4 to efde1cf Compare April 13, 2025 02:35

[clang-tidy] Add support for Initialization Forwarding in Nested Obje…

ed0c4bd

…cts within modernize-use-emplace

RiverDave force-pushed the modernize/125740 branch from efde1cf to ed0c4bd Compare May 3, 2025 04:46

vbvictor reviewed Jul 14, 2025

View reviewed changes

5chmidti reviewed Jul 14, 2025

View reviewed changes

		@@ -1,4 +1,12 @@
		// RUN: %check_clang_tidy %s modernize-use-emplace %t -- \
		// RUN: %check_clang_tidy %s -std=c++11 modernize-use-emplace %t -- \

	// RUN: %check_clang_tidy %s -std=c++11 modernize-use-emplace %t -- \
	// RUN: %check_clang_tidy %s -std=c++11,c++14,c++17 modernize-use-emplace %t -- \

	// RUN: %check_clang_tidy -std=c++20 %s modernize-use-emplace -check-suffixes=,CPP20 %t -- \
	// RUN: %check_clang_tidy -std=c++20-or-later %s modernize-use-emplace -check-suffixes=,CPP20 %t -- \

	if (InnerConstructorExpr && InnerConstructorExpr->getNumArgs() == 0) {
	if (InnerConstructorExpr->getNumArgs() == 0) {

	void CXX20testBracedInitTemporaries(){
	void CXX20testBracedInitTemporaries() {

[clang-tidy][C++20] Add support for Initialization Forwarding in structs and Nested Objects within modernize-use-emplace #131969

Are you sure you want to change the base?

[clang-tidy][C++20] Add support for Initialization Forwarding in structs and Nested Objects within modernize-use-emplace #131969

Conversation

RiverDave commented Mar 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Mar 19, 2025

Uh oh!

llvmbot commented Mar 19, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Mar 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RiverDave commented Mar 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RiverDave commented Mar 27, 2025

Uh oh!

vbvictor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

RiverDave commented Apr 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RiverDave commented Apr 18, 2025

Uh oh!

RiverDave commented May 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

RiverDave commented Mar 19, 2025 •

edited

Loading

github-actions bot commented Mar 24, 2025 •

edited

Loading

RiverDave commented Mar 24, 2025 •

edited

Loading

RiverDave commented Apr 5, 2025 •

edited

Loading